-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement no_blank_line_before_class_docstring
preview style
#9154
Conversation
|
It seems ecosystem reported numbers are wrong. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update your pr summary and include the 'old' compatibility numbers. I don't expect them to change but just to be safe
|
||
|
||
class Test: | ||
- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a few tests where we have comments before the docstring (including new or empty lines)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tested it out locally with different combinations of comments, newline and docstrings. It seems that the Black preview style is a bit inconsistent with the stable style.
Original source code:
class OneBlankLineDocstring:
"""This is a docstring."""
class DocstringWithComment0:
# This is a comment
"""This is a docstring."""
class DocstringWithComment1:
# This is a comment
"""This is a docstring."""
class DocstringWithComment2:
# This is a comment
"""This is a docstring."""
class DocstringWithComment3:
# This is a comment
"""This is a docstring."""
class DocstringWithComment4:
# This is a comment
"""This is a docstring."""
Black stable formatting:
class NormalDocstring:
"""This is a docstring."""
class DocstringWithComment0:
# This is a comment
"""This is a docstring."""
class DocstringWithComment1:
# This is a comment
"""This is a docstring."""
class DocstringWithComment2:
# This is a comment
"""This is a docstring."""
class DocstringWithComment3:
# This is a comment
"""This is a docstring."""
class DocstringWithComment4:
# This is a comment
"""This is a docstring."""
Black preview formatting:
class OneBlankLineDocstring:
"""This is a docstring."""
class DocstringWithComment0:
# This is a comment
"""This is a docstring."""
class DocstringWithComment1:
# This is a comment
"""This is a docstring."""
class DocstringWithComment2:
# This is a comment
"""This is a docstring."""
class DocstringWithComment3:
# This is a comment
"""This is a docstring."""
class DocstringWithComment4:
# This is a comment
"""This is a docstring."""
What I'm noticing is that the rules change if there's a comment in between the class header and docstring in a way that for n1
and n2
number of blank lines before a comment and docstring respectively the following holds true:
- If
n1
/n2
is 0, keep it 0 - If
n1
/n2
is 1, keep it 1 - If
n1
/n2
> 1, then reduce it to 1
It's probably another preview style which is keeping the newline before the comment (allow_empty_first_line_before_block_or_comment
).
The ecosystem line numbers are not guaranteed to be exact (hence the tilde). It’s a diff of two changes against HEAD, so mapping that diff back to a line number in HEAD is not trivial. (Not sure about the “number of lines changed” though.) |
779413d
to
2c8c450
Compare
Summary
This PR implements the
no_blank_line_before_class_docstring
preview style.Test Plan
Update existing snapshots.
Formatter ecosystem
main
dhruv/no-blank-line-docstring
fixes: #8888